test: remove sqlite skips for queue-worker tracing (merge after cds spawn fix) - #467
test: remove sqlite skips for queue-worker tracing (merge after cds spawn fix)#467sjvans wants to merge 6 commits into
Conversation
SummaryThe following content is AI-generated and provides a summary of the pull request:
Remove SQLite Skip Guards for Queue-Worker Tracing TestsTest🧪 Removes the temporary SQLite skip guards from 5 queue-worker tracing test suites that were added in #465 as a workaround for a Changes
Jira Issues (ONLY add this section if JIRA issues are available)N/A Related PRs
PR Bot InformationVersion:
|
There was a problem hiding this comment.
- Could not get issue
cap-js/telemetry#6594. Status: 404 - UnknownObjectException
The changes are straightforward — they remove temporary sqlite skip guards from five test files. Looking at the diff carefully:
-
In
tracing-messaging-inboxed.test.jsandtracing-messaging-outboxed-and-inboxed.test.js, theconst cds = require('@sap/cds')line is being removed along with the sqlite guard. However,cdsis not used anywhere else in those files — so the removal is clean. -
In
tracing-outboxed-batch.test.jsandtracing-scheduled.test.js,cdsis still used in the file (forcds.test(...),cds.version,cds.connect.to, etc.), so the removal of the guard is fine and does not leave a dangling require. -
In
tracing-messaging-persistent-outbox.test.js, same as the messaging files —cdsis not directly used in the file body outside what was removed.
The PR description is clear that this is intentionally gated on an upstream CDS fix and the checklist items are unresolved. There are no logic errors, bugs, or maintainability issues introduced by the diff itself — all the removals are consistent and correct.
This is a clean, intentional removal of temporary workaround guards. The diff is correct and all five files are consistent — no issues to flag. The open checklist items in the PR description (#465 merged and @sap/cds bumped) are the only remaining gates before merge, which are appropriately tracked in the PR description itself.
PR Bot Information
Version: 1.29.18
- LLM:
anthropic--claude-4.6-sonnet - Event Trigger:
pull_request.opened - Correlation ID:
9e2d0e20-919a-11f1-97de-effae0c5a313 - File Content Strategy: Full file content
60e48a9 to
b281c8f
Compare
9595503 to
31e0441
Compare
b281c8f to
9b88203
Compare
81c9cd9 to
5eb33aa
Compare
89d1e87 to
29710d5
Compare
5eb33aa to
5fe19e4
Compare
29710d5 to
3983e27
Compare
5fe19e4 to
179982c
Compare
… log-regex tests with structured in-memory spans
Queue worker tracing:
- Wraps cds.Service.prototype.tx() so that the queue worker's two-transaction
structure (SELECT+UPDATE lock tx, then handle+DELETE dispatch tx) appears as
child spans under their trace root instead of each top-level CAP call inside
them becoming an orphan root.
- Guard: skips when this.context instanceof cds.EventContext so $batch sub-requests
are not affected, and skips the bare {} context from processInboundMsg so
file-based messaging consumer delivery gets a messaging - tx root span.
- SQLite note: CDS uses a raw setTimeout bypass for sqlite queue workers (to avoid
deadlocks) so cds.spawn is not called, and spawn-root tracing is not available on
sqlite. Affected tests are skipped on sqlite and verified on HANA in CI.
Test infrastructure (from PR #450):
- MyInMemorySpanExporter: structured in-memory span store replacing fragile
cds.test.log() regex matching. groupedByTrace() / rootSpans() helpers let
tests assert on span trees directly.
- tracing-in-memory profile in .cdsrc.json; tracing-attributes profile preserved.
- tracing.test.js, tracing-attributes.test.js, tracing-mt.test.js,
tracing-messaging*.js rewritten to use structured span assertions.
- New test files: tracing-scheduled, tracing-outboxed-batch,
tracing-messaging-inboxed, tracing-messaging-outboxed-and-inboxed,
tracing-messaging-persistent-outbox, console-span-exporter (unit test).
- tracing-messaging.js: configurable waitMs (default 2500ms, 4000ms for
inbox/persistent-outbox scenarios); afterAll wait bumped to 2s.
- admin-service: test_outboxed_send / test_outboxed_send_batch / test_scheduled
actions added for the new test scenarios.
The queue-worker tracing tests (scheduled, outboxed-batch, and the inboxed/ outboxed-and-inboxed/persistent-outbox messaging cases) assert on the 'cds.spawn - run task' root span and its child tx spans. That root only appears when @sap/cds routes the sqlite queue worker through cds.spawn. Published cds uses a raw setTimeout bypass on sqlite (to avoid a single-writer deadlock), so those spans never appear there. Skip these suites on sqlite until the cds fix lands (cap/cds branch test/queue-spawn-sqlite-extended-tenant, which removes the bypass by fixing the actual deadlock root cause). A follow-up PR removes these skips once the required cds version is released. HANA CI already exercises the full path.
…quire, tighten mt assertion
- Number(cds.version.split('.')[0]) < 9 instead of relying on string coercion
(tracing-outboxed-batch, tracing-scheduled)
- hoist single hrTimeToNanoseconds require, drop the double require in the sort
comparator (tracing-outboxed-batch)
- multitenancy: assert exactly one AdminService READ span (filter + length 1)
instead of find, guarding against residue leaking past reset (tracing-mt)
Skipped bot findings: byParent[undefined] (field is unused by any test);
tx double-wrap guard (same pattern as existing emit/handle wraps, plugin
loads once per process).
3983e27 to
74d924a
Compare
179982c to
91c38f7
Compare
Messaging is outboxed by default, so `outboxed: true, inboxed: true` exercises the exact same code path and asserts the identical span shape as the `inboxed` test. Remove the duplicate test + its unused .cdsrc profile.
Removes the sqlite skip guards added while the cds queue-spawn fix was pending. With cds routing the sqlite queue worker through cds.spawn (cap/cds branch test/queue-spawn-sqlite-extended-tenant: removes the setTimeout bypass and fixes the underlying deadlock by wrapping ExtendedModels.model4() in cds.tx()), the 'cds.spawn - run task' root span and its child tx spans now appear on sqlite too. Merge only after that cds fix is released and the @sap/cds dependency is bumped.
91c38f7 to
d1812a1
Compare
…ucture (#465) ## Added Wraps `cds.Service.prototype.tx()` so the queue worker's two-transaction structure (tx1: SELECT+UPDATE lock, tx2: handle+DELETE dispatch) appears as coherent `<service> - tx` spans under the `cds.spawn - run task` root, instead of each top-level CAP call becoming an orphan root. Guarded so `$batch` sub-requests (active `EventContext`) are unaffected; file-based messaging consumer delivery (bare `{}` context) still gets a root span. ## Test infrastructure Replaces fragile `cds.test.log()` regex assertions with a structured in-memory span exporter (`MyInMemorySpanExporter`) and `groupedByTrace()` / `rootSpans()` helpers. Rewrites the existing tracing suites and adds coverage for scheduled tasks, outboxed batch fan-out, and inbox/outbox messaging combinations. ## SQLite note Queue-worker suites skip on sqlite (published `@sap/cds` uses a `setTimeout` bypass, not `cds.spawn`) — verified on HANA in CI. Follow-up #467 removes the skips once the cds queue-spawn fix ships. Changelog updated. Targets `develop`.
|
Part of the broader skip-elimination tracked in #477 (item 1). |
Depends on
@sap/cdsbumped: cap/cdstest/queue-spawn-sqlite-extended-tenant— removes the sqlitesetTimeoutbypass inlibx/queue/processing.js(_begin = cds.spawn.bind(cds)) and fixes the actual single-writer deadlock by wrappingExtendedModels.model4()incds.tx()insidelib/req/spawn.js.What
Removes the 5 sqlite skip guards added in #465. Once cds routes the sqlite queue worker through
cds.spawn, thecds.spawn - run taskroot span and its child tx spans appear on sqlite, so these suites verify the full path on sqlite (not just HANA):tracing-scheduled.test.jstracing-outboxed-batch.test.jstracing-messaging-inboxed.test.jstracing-messaging-outboxed-and-inboxed.test.jstracing-messaging-persistent-outbox.test.jsVerified
Locally against a workspace-linked
@sap/cdsattest/queue-spawn-sqlite-extended-tenant: all 5 suites pass on sqlite.Do not merge until
@sap/cdsdependency bumped in a preceding commit